Skip to content

offers: add used_count to listoffers#9162

Open
alexgrad42 wants to merge 2 commits into
ElementsProject:masterfrom
alexgrad42:feature/add_offer_used_count
Open

offers: add used_count to listoffers#9162
alexgrad42 wants to merge 2 commits into
ElementsProject:masterfrom
alexgrad42:feature/add_offer_used_count

Conversation

@alexgrad42
Copy link
Copy Markdown

This adds a new 'used_count' field to the 'listoffers' RPC command output to see the exact number of times a BOLT12 offer has been paid by counting associated settled invoices.

Changelog-Added: add a new 'used_count' field to the 'listoffers' RPC command

Closes #9146

@alexgrad42 alexgrad42 requested a review from cdecker as a code owner May 21, 2026 19:33
@alexgrad42 alexgrad42 force-pushed the feature/add_offer_used_count branch from 85af710 to e3719b3 Compare May 21, 2026 20:45
This adds a new 'used_count' field to the 'listoffers' RPC command
output to see the exact number of times a BOLT12 offer has been paid
by counting associated settled invoices.

Changelog-Added: add a new 'used_count' field to the 'listoffers' RPC command

Closes ElementsProject#9146
@alexgrad42 alexgrad42 force-pushed the feature/add_offer_used_count branch from e3719b3 to 5370032 Compare May 21, 2026 20:50
Copy link
Copy Markdown
Collaborator

@nGoline nGoline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great addition. This fills a real gap and the implementation is straightforward, the core idea is solid and it's close to being mergeable.
A few things to fix before we can land this.

Comment thread lightningd/offer.c
json_populate_offer(response,
&id, b12,
label, status, force_paths);
u64 used_count = wallet_offer_count_payments(wallet, offer_id);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're passing offer_id instead of &id here. Since we're inside the else block (meaning offer_id was not provided), this would pass NULL to a function annotated with NON_NULL_ARGS(1,2), so every offer in the list-all path would get a count of 0 (or worse).
Should be &id.

Comment thread wallet/wallet.h
Comment on lines +1517 to +1519
u64 wallet_offer_count_payments(struct wallet *w,
const struct sha256 *offer_id);
NON_NULL_ARGS(1,2);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should remove the ; between the function declaration and NON_NULL_ARGS.

Suggested change
u64 wallet_offer_count_payments(struct wallet *w,
const struct sha256 *offer_id);
NON_NULL_ARGS(1,2);
u64 wallet_offer_count_payments(struct wallet *w,
const struct sha256 *offer_id)
NON_NULL_ARGS(1,2);

Comment thread wallet/wallet.c
Comment on lines +6069 to +6071
" AND state = 1;"));

db_bind_sha256(stmt, offer_id);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded 1 is fragile and doesn't self-document intent. We have a enum constant for PAID.

Suggested change
" AND state = 1;"));
db_bind_sha256(stmt, offer_id);
" AND state = ?;"));
db_bind_sha256(stmt, offer_id);
db_bind_int(stmt, invoice_status_in_db(PAID));

Comment thread lightningd/offer.c
&id, b12,
label, status, force_paths);
u64 used_count = wallet_offer_count_payments(wallet, offer_id);
json_add_u64(response, "used_count", used_count);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation here is inconsistent with a mix of spaces and tabs. Please align it with the surrounding code (tabs only).

Comment thread .msggen.json
"deprecated": null
},
"ListOffers.offers[].used_count": {
"added": "pre-v0.10.1",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre-v0.10.1 is reserved for fields that existed before versioning was introduced. This is a new field, so it needs the correct release tag. There's no time to include it in v26.06, so let's target the next one: v26.09.

Comment on lines 41 to 48
"required": [
"offer_id",
"active",
"single_use",
"force_paths",
"bolt12",
"used"
],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used_count is always computed and returned unconditionally, so it should be listed in the required array of the items object in this schema.

Comment thread wallet/wallet.c
Comment on lines +6066 to +6069
stmt = db_prepare_v2(w->db, SQL("SELECT COUNT(*)"
" FROM invoices"
" WHERE local_offer_id = ?"
" AND state = 1;"));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This executes one SELECT COUNT(*) per offer, which means a node with 1000 offers will fire 1001 queries on every listoffers call. Consider computing all counts in a single query with GROUP BY local_offer_id, then looking up each offer's count from the result.

Comment thread wallet/wallet.c
stmt = db_prepare_v2(w->db, SQL("SELECT COUNT(*)"
" FROM invoices"
" WHERE local_offer_id = ?"
" AND state = 1;"));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace: make check-source will catch this. Please remove the space after ));..

Comment thread lightningd/offer.c
&id, b12,
label, status, force_paths);
u64 used_count = wallet_offer_count_payments(wallet, offer_id);
json_add_u64(response, "used_count", used_count);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're adding this field between json_populate_offer() output and description. Consider moving it after description so the JSON output is consistent between versions.

Comment thread tests/test_pay.py
Comment on lines +7347 to +7348
assert 'used_count' in offers[0], "Error: used_count field is missing from listoffers response!"
assert offers[0]['used_count'] == 0
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's drop the custom message string. The in check is redundant since the == 0 assertion on the next line would already produce a KeyError if the key were missing. Plain assert is the project convention here.

Suggested change
assert 'used_count' in offers[0], "Error: used_count field is missing from listoffers response!"
assert offers[0]['used_count'] == 0
assert offers[0]['used_count'] == 0

@nGoline
Copy link
Copy Markdown
Collaborator

nGoline commented May 22, 2026

Please rebase onto master instead of merging it in. Merging master into your feature branch creates a noisy history and makes the diff harder to follow. A clean rebase keeps the commits linear and makes it easier to bisect if something goes wrong later

@nGoline
Copy link
Copy Markdown
Collaborator

nGoline commented May 22, 2026

Please rebase onto master instead of merging it in. Merging master into your feature branch creates a noisy history and makes the diff harder to follow. A clean rebase keeps the commits linear and makes it easier to bisect if something goes wrong later

Also, when addressing the review feedback, please use fixup! commits (or git commit --fixup=<sha>) rather than separate "fix" commits. Having a commit like "fix indentation" or "address review comments" in the final history makes it harder to bisect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BOLT12: getting how many times an offer was used

2 participants